I think the iter.partitionLevelDeletions gc-able is kinda checked with the cell.isLive below, but there may be partition level delete with no actual data behind it in other sstable that wouldn't be counted. Worth a test at least

public class TestTest extends CQLTester
{
    @BeforeClass
    public static void setUpClass()
    {
        CQLTester.setUpClass();
    }
    @Test
    public void test() throws Throwable
    {
        String table = createTable("CREATE TABLE %s (a int, b text, PRIMARY KEY (a)) WITH gc_grace_seconds = 0");
        execute("DELETE FROM %s WHERE a = 1");
        Util.flushTable(KEYSPACE, table);
        Thread.sleep(1000);
        Assert.assertEquals(0, ColumnFamilyStore.getIfExists(KEYSPACE, table).metric.purgeableTombstoneScannedHistogram.getCount());
        execute("SELECT * FROM %s");
        Assert.assertEquals(1, ColumnFamilyStore.getIfExists(KEYSPACE, table).metric.purgeableTombstoneScannedHistogram.getCount());
        Assert.assertEquals(1, ColumnFamilyStore.getIfExists(KEYSPACE, table).metric.purgeableTombstoneScannedHistogram.getSnapshot().getMax());
    }
}

fails for example when considerZeroes is true

with

            public UnfilteredRowIterator applyToPartition(UnfilteredRowIterator iter)
            {
                if (!iter.partitionLevelDeletion().isLive())
                    purgeableTombstones++;

above passes

Hm, interesting. Thank you for noticing it. I have created PurgeableTombstonesMetricRecording iterator based on MetricRecording iterator logic. It looks like the existing tombstoneScannedHistogram has the same behaviour. Should I change considerZeroes = true for tombstoneScannedHistogram and add the counter increment on a partition delete to MetricRecording as well?

I think we should update existing one and put a note in NEWS.txt, but might be worth a discuss on dev list. It can be a bit confusing when there is, or isnt data shadowed

I have looked again through the added code and now I am a bit concerned about the iteration over cells to check for cell tombstones - it may create a performance overhead + I am not sure if information about droppable cell tombstones is really valuable: all the times when I saw issues with droppable tombstones - it was about row tombstones. I am thinking now about an option to count only partition level and row level tombstones in this added logic and skip cell tombstones part..
I am going to measure the overhead to see how big the impact can be..

good to measure but the cells are already materialized so the overhead id imagine to be pretty minor, could maybe piggy bank on the withMetricsRecording's iterations (like countTombstone method)

yes, I do not expect a huge impact but I want to check if it is visible. We may read many rows if we select a big enough partition and each row has some number of cells, so MxN cells to iterate and check. As I mentioned, I suppose frequently per-cell stats are not really needed to pay an overhead for it, IMHO.. A configuration option can be a compromise here, like: droppable_tobmstones_metric_granularity: disabled|row|cell

In https://issues.apache.org/jira/browse/CASSANDRA-20165 I have spent some time already to try to reduce the number of such non-optimal places already, so I would not like to introduce one more by myself :-D

I have added results to CASSANDRA-20132

public class TestTest extends CQLTester
{
    @BeforeClass
    public static void setUpClass()
    {
        CQLTester.setUpClass();
    }
    @Test
    public void test() throws Throwable
    {
        String table = createTable("CREATE TABLE %s (a int, b text, PRIMARY KEY (a)) WITH gc_grace_seconds = 0");
        execute("DELETE FROM %s WHERE a = 1");
        Util.flushTable(KEYSPACE, table);
        Thread.sleep(1000);
        Assert.assertEquals(0, ColumnFamilyStore.getIfExists(KEYSPACE, table).metric.purgeableTombstoneScannedHistogram.getCount());
        execute("SELECT * FROM %s");
        Assert.assertEquals(1, ColumnFamilyStore.getIfExists(KEYSPACE, table).metric.purgeableTombstoneScannedHistogram.getCount());
        Assert.assertEquals(1, ColumnFamilyStore.getIfExists(KEYSPACE, table).metric.purgeableTombstoneScannedHistogram.getSnapshot().getMax());
    }
}

fails for example when considerZeroes is true

with

            public UnfilteredRowIterator applyToPartition(UnfilteredRowIterator iter)
            {
                if (!iter.partitionLevelDeletion().isLive())
                    purgeableTombstones++;

above passes